Skip to content

Add tpl support for ingress.labels, ingress.extraPaths and nodeSelector#400

Open
treydock wants to merge 8 commits intooauth2-proxy:mainfrom
treydock:tpl-ingress
Open

Add tpl support for ingress.labels, ingress.extraPaths and nodeSelector#400
treydock wants to merge 8 commits intooauth2-proxy:mainfrom
treydock:tpl-ingress

Conversation

@treydock
Copy link
Copy Markdown
Contributor

@treydock treydock commented Mar 25, 2026

Description

Add deploymentLabels

Allow templated ingress labels, ingress extraPaths and nodeSelector

Checklist:

  • I have bumped the version in the Chart.yaml according to Semantic Versioning.
  • I have updated the documentation/CHANGELOG at the bottom of the Chart.yaml
  • I have signed off all my commits.
  • (Optional) I have updated the Chart.lock for dependency updates
  • (Optional) I have implemented helm tests for new feature flags

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the helm/oauth2-proxy chart’s Ingress templating capabilities by allowing Helm tpl evaluation within ingress.labels and ingress.extraPaths, and updates CI values and chart metadata accordingly.

Changes:

  • Add tpl rendering for ingress.labels and ingress.extraPaths in the Ingress template.
  • Extend the CI tpl-values.yaml to cover templated ingress.labels and ingress.extraPaths.
  • Bump chart version and update the ArtifactHub changelog entry/link.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
helm/oauth2-proxy/templates/ingress.yaml Wraps ingress.labels and ingress.extraPaths YAML rendering with tpl to support templated values.
helm/oauth2-proxy/ci/tpl-values.yaml Adds CI coverage values for the new templated Ingress fields.
helm/oauth2-proxy/Chart.yaml Version bump and ArtifactHub changelog update referencing this PR.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pierluigilenoci
Copy link
Copy Markdown
Member

@treydock, can you check the Copilot conversation?

@treydock
Copy link
Copy Markdown
Contributor Author

@pierluigilenoci I think some of the Copilot suggestions were incorrect but one suggestion was made to use nindent and strip leading spaces so made that change to both places I modified.

Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@treydock treydock changed the title Add tpl support for ingress.labels and ingress.extraPaths Add tpl support for ingress.labels, ingress.extraPaths and nodeSelector Mar 26, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
@treydock
Copy link
Copy Markdown
Contributor Author

@pierluigilenoci Let me know if any additional changes are required

@pierluigilenoci pierluigilenoci requested a review from Copilot March 29, 2026 14:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tuunit
Copy link
Copy Markdown
Member

tuunit commented Mar 31, 2026

Could we stop opening a single PRs for each next element we want to add tpl to? Could we please just open one large PR for this?

@treydock
Copy link
Copy Markdown
Contributor Author

There was one large PR, but I missed a few things as I had intended to only open 1 PR. I can expand this PR to use tpl in more places but that becomes a challenge if every tpl usage requires some kind of testing values change as likely not all things can be tested with simple chart testing deployments on Github Actions.

@treydock
Copy link
Copy Markdown
Contributor Author

@tuunit Let me know if you'd like me to update this PR to template most/all values with tpl.

@treydock
Copy link
Copy Markdown
Contributor Author

treydock commented Apr 2, 2026

@pierluigilenoci @tuunit Let me know if you'd like me to expand more values to use tpl or if this can be merged as-is.

Signed-off-by: Trey Dockendorf <tdockendorf@osc.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants